feat: Improve error messages with actionable guidance#3
Conversation
- Extract and display error details from API response body - Add specific guidance for common errors: - Nested archives: suggest using .gitattributes export-ignore - Size limits: suggest excluding large files - Rate limits: suggest waiting before retry - Handle 401, 413, 429, and 500 status codes with helpful messages
WalkthroughUpdated Changes
Sequence Diagram(s)sequenceDiagram
participant Action as Action
participant API as External API
participant Handler as Error Handler (src/index.ts)
participant Logger as GitHub Actions core
Action->>API: make request
API-->>Action: responds with error (status, body, headers)
Action->>Handler: catch error
Handler->>Handler: safeSerialize & redactSensitive(error/response)
Handler->>Handler: determine status & apiMessage from multiple fields
Handler->>Logger: core.debug/core.info structured logs (message, name, status, body, headers)
Handler->>Logger: core.setFailed(errorMessage + optional helpText)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🔇 Additional comments (6)
✏️ Tip: You can disable this entire section by setting Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/index.ts`:
- Around line 123-148: The current error-logging block uses core.info and direct
JSON.stringify calls that can leak secrets (Authorization/Cookie) and throw on
circular/BigInt values; change logging to use core.debug for sensitive details,
replace direct JSON.stringify calls with a safeSerialize helper (that catches
stringify errors and returns a deterministic placeholder like "[unserializable]"
or truncates large values), and redact sensitive header/body fields (e.g.,
Authorization, Cookie, set-cookie, access_token) before serializing; ensure all
uses in this block (references: core.info, JSON.stringify, error,
error.response, error.response.headers, error.response.data, error.body,
error.cause) are wrapped so they never throw and never log raw secret values.
🧹 Nitpick comments (1)
src/index.ts (1)
178-187: Make the 500‑error heuristics case‑insensitive.API messages can vary in casing; lowercasing once avoids missing matches.
♻️ Small tweak
- } else if (status === 500) { + } else if (status === 500) { errorMessage = apiMessage || 'Internal server error'; // Check for common issues and provide guidance - if (apiMessage.includes('Nested archives')) { + const msg = apiMessage.toLowerCase(); + if (msg.includes('nested archives')) { helpText = 'Your repository contains nested archive files (.zip, .tar, etc.). ' + 'Add them to .gitattributes with "export-ignore" to exclude from analysis. ' + 'Example: tests/fixtures/*.zip export-ignore'; - } else if (apiMessage.includes('exceeds maximum')) { + } else if (msg.includes('exceeds maximum')) { helpText = 'Your repository or a file within it exceeds size limits. ' + 'Consider excluding large files using .gitattributes with "export-ignore".'; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (1)
src/index.ts
🔇 Additional comments (1)
src/index.ts (1)
150-199: Nice, resilient message extraction chain.The multi-source
apiMessagefallback plus status-based messaging is clear and should make errors much more actionable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Add safeSerialize helper to handle circular refs, BigInt, and truncate large values - Add redactSensitive helper to redact Authorization, Cookie, token fields - Use core.debug for sensitive details (headers, cause) - Use core.info for safe error summaries (status, message, data) - Wrap all serialization in try/catch to prevent throws - Add SENSITIVE_KEYS set for consistent redaction
Summary
.gitattributes export-ignoreContext
This addresses the issue where users see generic "API error (500)" without understanding the root cause. The most common cause is nested archive files (
.zip,.tar, etc.) triggering the API's zip bomb protection.Test plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.